std.os.uefi.protocol: ziggify function signatures #23214
Conversation
dotcarmen
force-pushed
the
uefi-fn-signatures
branch
from
4f65107 to
c5ad247
Compare
|
one problem I'm struggling to address in this PR is the potential for the underlying API to return status codes - most of std.os.uefi already assumes non- |
dotcarmen
force-pushed
the
uefi-fn-signatures
branch
from
c5ad247 to
3c4cfa8
Compare
dotcarmen
force-pushed
the
uefi-fn-signatures
branch
from
3c4cfa8 to
29c91b7
Compare
| pub fn delete(self: *File) bool { | ||
| switch (self._delete(self)) { | ||
| .success => return true, | ||
| .warn_delete_failure => return false, |
There was a problem hiding this comment.
That seems like a good solution. Once we find an API that can return both a value and a warning it'll get awkward though.
| } | ||
|
|
||
| /// Polls for incoming data packets and processes outgoing data packets. | ||
| pub fn poll(self: *const Ip6) Status { | ||
| return self._poll(self); | ||
| pub fn poll(self: *Ip6) PollError!void { |
There was a problem hiding this comment.
i feel like this should return PollError!bool, where .success is true and .not_ready is false - wdyt? according to the spec, poll returns .not_ready when no data is processed, as opposed to .success when data is processed
| data_type: DataType, | ||
| data_size: usize, | ||
| data: *const anyopaque, |
There was a problem hiding this comment.
the data type should probably be a union (enum) eventually, but i'm concerned about how that would affect getData where the out pointer is the field defined by data_type from the union
| pub const CreateChildError = uefi.UnexpectedError || error{ | ||
| InvalidParameter, | ||
| OutOfResources, | ||
| } || Error; // TODO: according to the spec, _any other_ status is returnable? |
There was a problem hiding this comment.
i only did a little bit of google, but couldn't figure out what Other means in the spec. i have a feeling it's referring to warnings, but i'm wary of assuming such and i think i'd rather let somebody who encounters this file their own bug report/pr to fix...
| // .invalid_parameter => return Error.InvalidParameter, | ||
| // .unsupported => return Error.Unsupported, | ||
| // .not_started => return Error.NotStarted, | ||
| else => |status| { | ||
| try status.err(); | ||
| // TODO: only warnings get here I think? | ||
| return uefi.unexpectedStatus(status); |
There was a problem hiding this comment.
depending on how we decide on approaching Other above, i'll change the logic here and below
|
for warnings, i'm considering a global alternatively, I can make a |
| if (size_of_info != @sizeOf(Mode.Info)) | ||
| return error.Unexpected |
There was a problem hiding this comment.
also thoughts on this pattern? the 3rd parameter is for informing the caller of the implementation's sizeOf(Mode.Info) even though the struct is well-defined according to the spec...
this also applies to SimpleNetwork.statistics
A lot of the function signatures in
std.os.uefi.protocolaren't very ziggy. This PR modifies a significant number of the namespace's functions to align more with verbose Zig code.*to*constand vice-versa as appropriateuefi.unexpectedError, which is akin toposix.unexpectedError.success, rather than requiring an output pointer parameternote: most of
std.os.uefiis untested, and that's not really changing here. I'm mostly focused on making sure there are no compiler errors. Any further problems will require a future PR to address.